router-tests/modules/entity-splitter/module.go (2)

291-299: Response body is read before status check, wasting I/O on errors.

The body is fully read (line 291) before checking status code (line 296). For non-200 responses, this reads potentially large error bodies unnecessarily.

♻️ Minor optimization

 	resp, err := m.Client.Do(subReq)
 	if err != nil {
 		out.fetchErr = fmt.Errorf("sub-fetch: %w", err)
 		return
 	}
 	defer resp.Body.Close()

+	if resp.StatusCode != http.StatusOK {
+		out.fetchErr = fmt.Errorf("sub-fetch returned status %d", resp.StatusCode)
+		return
+	}
+
 	bodyBytes, err := io.ReadAll(resp.Body)
 	if err != nil {
 		out.fetchErr = fmt.Errorf("read sub-response: %w", err)
 		return
 	}
-	if resp.StatusCode != http.StatusOK {
-		out.fetchErr = fmt.Errorf("sub-fetch returned status %d", resp.StatusCode)
-		return
-	}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@router-tests/modules/entity-splitter/module.go` around lines 291 - 299, The
code reads resp.Body into bodyBytes before checking resp.StatusCode, causing
unnecessary I/O for non-200 responses; change the logic in the fetch handling
(the block using resp, bodyBytes and out.fetchErr) to first inspect
resp.StatusCode and set out.fetchErr for non-OK statuses without reading the
full body, and only call io.ReadAll(resp.Body) when resp.StatusCode ==
http.StatusOK (optionally reading a small/truncated body for error logging if
needed) so you avoid reading large error payloads unnecessarily.

---

241-253: Prefer sync.WaitGroup.Go for cleaner goroutine management.

Go 1.25+ provides wg.Go(func()) which handles Add/Done automatically. This is the preferred pattern in this repository.

♻️ Proposed refactor

-	var wg sync.WaitGroup
-	wg.Add(len(chunks))
 	absStart := 0
+	var wg sync.WaitGroup
 	for i, chunk := range chunks {
 		results[i].absStart = absStart
 		results[i].chunkLen = len(chunk)
 		absStart += len(chunk)
-		go func(idx int, chunkReps [][]byte) {
-			defer wg.Done()
+		wg.Go(func() {
 			m.SubFetchCount.Add(1)
-			m.runSubFetch(req, pr, chunkReps, &results[idx])
-		}(i, chunk)
+			m.runSubFetch(req, pr, chunk, &results[i])
+		})
 	}
 	wg.Wait()

Based on learnings: "In Go code (Go 1.25+), prefer using sync.WaitGroup.Go(func()) to run a function in a new goroutine, letting the WaitGroup manage Add/Done automatically."

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@router-tests/modules/entity-splitter/module.go` around lines 241 - 253,
Replace the manual wg.Add(len(chunks)) + go func(...){ defer wg.Done() ... }
pattern with sync.WaitGroup.Go for each chunk: remove the wg.Add call, and
inside the loop call wg.Go(func() { m.SubFetchCount.Add(1); m.runSubFetch(req,
pr, chunkReps, &results[idx]) }) while ensuring you capture loop variables
safely (pass idx and chunk as parameters or assign to locals like i2, chunk2) so
m.runSubFetch and results[i] use the correct values; keep existing assignments
to results[i].absStart and results[i].chunkLen before launching the goroutine.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes